-
Notifications
You must be signed in to change notification settings - Fork 314
support service discovery with JNA #9705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
🎯 Code Coverage 🔗 Commit SHA: fa05b36 | Docs | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 56 metrics, 9 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.55.0-SNAPSHOT~fa05b36659, baseline=1.55.0-SNAPSHOT~c85d09f004
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.019 s) : 0, 1018865
Total [baseline] (10.685 s) : 0, 10685259
Agent [candidate] (1.025 s) : 0, 1024999
Total [candidate] (10.831 s) : 0, 10831201
section appsec
Agent [baseline] (1.195 s) : 0, 1194750
Total [baseline] (11.097 s) : 0, 11097424
Agent [candidate] (1.202 s) : 0, 1201617
Total [candidate] (10.839 s) : 0, 10839238
section iast
Agent [baseline] (1.158 s) : 0, 1157750
Total [baseline] (11.082 s) : 0, 11081535
Agent [candidate] (1.15 s) : 0, 1150424
Total [candidate] (11.125 s) : 0, 11124772
section profiling
Agent [baseline] (1.163 s) : 0, 1163190
Total [baseline] (11.023 s) : 0, 11023431
Agent [candidate] (1.172 s) : 0, 1171839
Total [candidate] (10.941 s) : 0, 10940510
gantt
title petclinic - break down per module: candidate=1.55.0-SNAPSHOT~fa05b36659, baseline=1.55.0-SNAPSHOT~c85d09f004
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.471 ms) : 0, 1471
crashtracking [candidate] (1.483 ms) : 0, 1483
BytebuddyAgent [baseline] (694.199 ms) : 0, 694199
BytebuddyAgent [candidate] (697.693 ms) : 0, 697693
GlobalTracer [baseline] (242.065 ms) : 0, 242065
GlobalTracer [candidate] (243.979 ms) : 0, 243979
AppSec [baseline] (32.424 ms) : 0, 32424
AppSec [candidate] (32.831 ms) : 0, 32831
Debugger [baseline] (6.45 ms) : 0, 6450
Debugger [candidate] (6.442 ms) : 0, 6442
Remote Config [baseline] (686.49 µs) : 0, 686
Remote Config [candidate] (679.145 µs) : 0, 679
Telemetry [baseline] (9.295 ms) : 0, 9295
Telemetry [candidate] (9.546 ms) : 0, 9546
Flare Poller [baseline] (11.188 ms) : 0, 11188
Flare Poller [candidate] (11.103 ms) : 0, 11103
section appsec
crashtracking [baseline] (1.463 ms) : 0, 1463
crashtracking [candidate] (1.458 ms) : 0, 1458
BytebuddyAgent [baseline] (718.095 ms) : 0, 718095
BytebuddyAgent [candidate] (722.883 ms) : 0, 722883
GlobalTracer [baseline] (234.267 ms) : 0, 234267
GlobalTracer [candidate] (236.465 ms) : 0, 236465
IAST [baseline] (24.798 ms) : 0, 24798
IAST [candidate] (24.999 ms) : 0, 24999
AppSec [baseline] (174.891 ms) : 0, 174891
AppSec [candidate] (175.459 ms) : 0, 175459
Debugger [baseline] (6.192 ms) : 0, 6192
Debugger [candidate] (6.053 ms) : 0, 6053
Remote Config [baseline] (636.521 µs) : 0, 637
Remote Config [candidate] (620.477 µs) : 0, 620
Telemetry [baseline] (8.554 ms) : 0, 8554
Telemetry [candidate] (8.527 ms) : 0, 8527
Flare Poller [baseline] (4.805 ms) : 0, 4805
Flare Poller [candidate] (3.935 ms) : 0, 3935
section iast
crashtracking [baseline] (1.461 ms) : 0, 1461
crashtracking [candidate] (1.458 ms) : 0, 1458
BytebuddyAgent [baseline] (817.928 ms) : 0, 817928
BytebuddyAgent [candidate] (814.404 ms) : 0, 814404
GlobalTracer [baseline] (234.146 ms) : 0, 234146
GlobalTracer [candidate] (232.077 ms) : 0, 232077
IAST [baseline] (27.088 ms) : 0, 27088
IAST [candidate] (26.498 ms) : 0, 26498
AppSec [baseline] (35.59 ms) : 0, 35590
AppSec [candidate] (34.966 ms) : 0, 34966
Debugger [baseline] (6.24 ms) : 0, 6240
Debugger [candidate] (6.167 ms) : 0, 6167
Remote Config [baseline] (627.199 µs) : 0, 627
Remote Config [candidate] (595.415 µs) : 0, 595
Telemetry [baseline] (8.839 ms) : 0, 8839
Telemetry [candidate] (8.574 ms) : 0, 8574
Flare Poller [baseline] (4.339 ms) : 0, 4339
Flare Poller [candidate] (4.246 ms) : 0, 4246
section profiling
crashtracking [baseline] (1.429 ms) : 0, 1429
crashtracking [candidate] (1.446 ms) : 0, 1446
BytebuddyAgent [baseline] (722.465 ms) : 0, 722465
BytebuddyAgent [candidate] (727.042 ms) : 0, 727042
GlobalTracer [baseline] (217.674 ms) : 0, 217674
GlobalTracer [candidate] (220.078 ms) : 0, 220078
AppSec [baseline] (32.385 ms) : 0, 32385
AppSec [candidate] (32.711 ms) : 0, 32711
Debugger [baseline] (6.551 ms) : 0, 6551
Debugger [candidate] (8.414 ms) : 0, 8414
Remote Config [baseline] (767.579 µs) : 0, 768
Remote Config [candidate] (720.467 µs) : 0, 720
Telemetry [baseline] (15.198 ms) : 0, 15198
Telemetry [candidate] (14.617 ms) : 0, 14617
Flare Poller [baseline] (4.866 ms) : 0, 4866
Flare Poller [candidate] (4.194 ms) : 0, 4194
ProfilingAgent [baseline] (108.991 ms) : 0, 108991
ProfilingAgent [candidate] (109.681 ms) : 0, 109681
Profiling [baseline] (109.743 ms) : 0, 109743
Profiling [candidate] (110.294 ms) : 0, 110294
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.55.0-SNAPSHOT~fa05b36659, baseline=1.55.0-SNAPSHOT~c85d09f004
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.024 s) : 0, 1024233
Total [baseline] (8.667 s) : 0, 8666831
Agent [candidate] (1.027 s) : 0, 1026688
Total [candidate] (8.705 s) : 0, 8704840
section iast
Agent [baseline] (1.152 s) : 0, 1152209
Total [baseline] (9.296 s) : 0, 9296380
Agent [candidate] (1.154 s) : 0, 1154356
Total [candidate] (9.304 s) : 0, 9304436
gantt
title insecure-bank - break down per module: candidate=1.55.0-SNAPSHOT~fa05b36659, baseline=1.55.0-SNAPSHOT~c85d09f004
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.479 ms) : 0, 1479
crashtracking [candidate] (1.478 ms) : 0, 1478
BytebuddyAgent [baseline] (697.618 ms) : 0, 697618
BytebuddyAgent [candidate] (698.59 ms) : 0, 698590
GlobalTracer [baseline] (242.764 ms) : 0, 242764
GlobalTracer [candidate] (244.791 ms) : 0, 244791
AppSec [baseline] (32.832 ms) : 0, 32832
AppSec [candidate] (32.915 ms) : 0, 32915
Debugger [baseline] (6.555 ms) : 0, 6555
Debugger [candidate] (6.548 ms) : 0, 6548
Remote Config [baseline] (700.726 µs) : 0, 701
Remote Config [candidate] (695.095 µs) : 0, 695
Telemetry [baseline] (9.398 ms) : 0, 9398
Telemetry [candidate] (9.542 ms) : 0, 9542
Flare Poller [baseline] (11.702 ms) : 0, 11702
Flare Poller [candidate] (10.947 ms) : 0, 10947
section iast
crashtracking [baseline] (1.489 ms) : 0, 1489
crashtracking [candidate] (1.486 ms) : 0, 1486
BytebuddyAgent [baseline] (816.557 ms) : 0, 816557
BytebuddyAgent [candidate] (816.567 ms) : 0, 816567
GlobalTracer [baseline] (231.763 ms) : 0, 231763
GlobalTracer [candidate] (232.836 ms) : 0, 232836
IAST [baseline] (26.323 ms) : 0, 26323
IAST [candidate] (26.713 ms) : 0, 26713
AppSec [baseline] (34.999 ms) : 0, 34999
AppSec [candidate] (35.38 ms) : 0, 35380
Debugger [baseline] (6.094 ms) : 0, 6094
Debugger [candidate] (6.207 ms) : 0, 6207
Remote Config [baseline] (614.057 µs) : 0, 614
Remote Config [candidate] (597.224 µs) : 0, 597
Telemetry [baseline] (8.678 ms) : 0, 8678
Telemetry [candidate] (8.787 ms) : 0, 8787
Flare Poller [baseline] (4.219 ms) : 0, 4219
Flare Poller [candidate] (4.282 ms) : 0, 4282
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 1 performance regressions! Performance is the same for 9 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~fa05b36659, baseline=1.55.0-SNAPSHOT~c85d09f004
dateFormat X
axisFormat %s
section baseline
no_agent (4.177 ms) : 4127, 4227
. : milestone, 4177,
iast (10.536 ms) : 10351, 10720
. : milestone, 10536,
iast_FULL (14.526 ms) : 14234, 14818
. : milestone, 14526,
iast_GLOBAL (10.528 ms) : 10338, 10717
. : milestone, 10528,
profiling (9.212 ms) : 9056, 9368
. : milestone, 9212,
tracing (7.73 ms) : 7614, 7845
. : milestone, 7730,
section candidate
no_agent (4.158 ms) : 4108, 4207
. : milestone, 4158,
iast (9.567 ms) : 9407, 9727
. : milestone, 9567,
iast_FULL (14.303 ms) : 14022, 14585
. : milestone, 14303,
iast_GLOBAL (10.822 ms) : 10628, 11015
. : milestone, 10822,
profiling (8.746 ms) : 8610, 8882
. : milestone, 8746,
tracing (7.557 ms) : 7442, 7672
. : milestone, 7557,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~fa05b36659, baseline=1.55.0-SNAPSHOT~c85d09f004
dateFormat X
axisFormat %s
section baseline
no_agent (37.264 ms) : 36962, 37566
. : milestone, 37264,
appsec (48.457 ms) : 48010, 48904
. : milestone, 48457,
code_origins (45.474 ms) : 45069, 45878
. : milestone, 45474,
iast (46.059 ms) : 45666, 46452
. : milestone, 46059,
profiling (47.593 ms) : 47128, 48059
. : milestone, 47593,
tracing (45.169 ms) : 44781, 45557
. : milestone, 45169,
section candidate
no_agent (36.541 ms) : 36242, 36840
. : milestone, 36541,
appsec (48.846 ms) : 48409, 49282
. : milestone, 48846,
code_origins (44.303 ms) : 43925, 44681
. : milestone, 44303,
iast (44.937 ms) : 44556, 45318
. : milestone, 44937,
profiling (49.301 ms) : 48792, 49810
. : milestone, 49301,
tracing (44.695 ms) : 44316, 45075
. : milestone, 44695,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~fa05b36659, baseline=1.55.0-SNAPSHOT~c85d09f004
dateFormat X
axisFormat %s
section baseline
no_agent (1.476 ms) : 1465, 1488
. : milestone, 1476,
appsec (3.685 ms) : 3468, 3902
. : milestone, 3685,
iast (2.21 ms) : 2147, 2274
. : milestone, 2210,
iast_GLOBAL (2.254 ms) : 2190, 2318
. : milestone, 2254,
profiling (2.052 ms) : 2000, 2103
. : milestone, 2052,
tracing (2.032 ms) : 1982, 2082
. : milestone, 2032,
section candidate
no_agent (1.473 ms) : 1462, 1485
. : milestone, 1473,
appsec (3.693 ms) : 3475, 3911
. : milestone, 3693,
iast (2.199 ms) : 2135, 2262
. : milestone, 2199,
iast_GLOBAL (2.242 ms) : 2178, 2306
. : milestone, 2242,
profiling (2.047 ms) : 1996, 2098
. : milestone, 2047,
tracing (2.016 ms) : 1967, 2065
. : milestone, 2016,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~fa05b36659, baseline=1.55.0-SNAPSHOT~c85d09f004
dateFormat X
axisFormat %s
section baseline
no_agent (14.945 s) : 14945000, 14945000
. : milestone, 14945000,
appsec (15.037 s) : 15037000, 15037000
. : milestone, 15037000,
iast (18.606 s) : 18606000, 18606000
. : milestone, 18606000,
iast_GLOBAL (18.166 s) : 18166000, 18166000
. : milestone, 18166000,
profiling (15.626 s) : 15626000, 15626000
. : milestone, 15626000,
tracing (15.139 s) : 15139000, 15139000
. : milestone, 15139000,
section candidate
no_agent (14.913 s) : 14913000, 14913000
. : milestone, 14913000,
appsec (14.875 s) : 14875000, 14875000
. : milestone, 14875000,
iast (18.703 s) : 18703000, 18703000
. : milestone, 18703000,
iast_GLOBAL (18.047 s) : 18047000, 18047000
. : milestone, 18047000,
profiling (15.263 s) : 15263000, 15263000
. : milestone, 15263000,
tracing (15.07 s) : 15070000, 15070000
. : milestone, 15070000,
|
f852d0d to
a46f9e5
Compare
d8eb447 to
65ffa65
Compare
dd-trace-core/src/main/java/datadog/trace/core/ServiceDiscovery.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/ServiceDiscovery.java
Outdated
Show resolved
Hide resolved
|
@raphaelgavache iiuc GraalVM and Spring Native are not expected to work because of the way native libraries work with native image. So long as it won't crash or cause other adverse behavior in those scenarios, I think we're good |
Yes, Graal native image support is awhile off for any native library usage. |
| public class ServiceDiscovery { | ||
| private static final Logger log = LoggerFactory.getLogger(ServiceDiscovery.class); | ||
|
|
||
| private static final byte[] SCHEMA_VERSION = "schema_version".getBytes(ISO_8859_1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this particular case, I'd rather not have the constants.
The problem is that they end up permanently consuming memory when we really only intend to use them once.
Admittedly, this is a weird case and it has got me thinking about whether we want to just unload the class, but that's on platform to figure out.
| TracerVersion.TRACER_VERSION, | ||
| config.getHostName(), | ||
| config.getRuntimeId(), | ||
| config.getServiceName(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is using the statically configured service name which isn't necessarily the service name that the tracer will use in the end. Are we okay with that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might also want to consider moving this to a background task, so it doesn't impact start-up.
| mapElements += (processTags != null && processTags.length() > 0) ? 1 : 0; | ||
| mapElements += (containerID != null && !containerID.isEmpty()) ? 1 : 0; | ||
|
|
||
| SimpleUtf8Cache encodingCache = new SimpleUtf8Cache(256); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would skip the cache in this case.
Since the code is creating a new cache each time, the cache isn't providing a benefit here. And the cache is actually slower than just doing the encoding directly, the cache is helpful in rducing allocation if we're repeatedly encoding again and again.
But since this code is only called a handful of times, there's not much chance to save on allocation.
|
I think that the write should be done asynchronously (not on the premain) since the startup time skyrocketed. Ideally also the related classloading can be deferred on a scheduled task to be done after the tracer started |
f41114c to
bdc4e71
Compare
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
dd-trace-core/src/main/java/datadog/trace/core/servicediscovery/ServiceDiscovery.java
Show resolved
Hide resolved
924872b to
382c311
Compare
| static final boolean DEFAULT_TELEMETRY_LOG_COLLECTION_ENABLED = true; | ||
| static final int DEFAULT_TELEMETRY_DEPENDENCY_RESOLUTION_QUEUE_SIZE = 100000; | ||
|
|
||
| static final boolean DEFAULT_SERVICE_DISCOVERY_ENABLED = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❔ question: Are there some other products doing JNA at startup by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know about the JNA part but the memfd is enabled by default on all tracers except java for multiple months now. Agent products consume it
dd-trace-core/src/main/java/datadog/trace/core/servicediscovery/ForeignMemoryWriter.java
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/servicediscovery/ServiceDiscoveryFactory.java
Outdated
Show resolved
Hide resolved
| tagInterceptor, | ||
| strictTraceWrites, | ||
| instrumentationGateway, | ||
| null, // you might refactor this as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❔ question: left over? Have a noop instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just removed the comment, I'm not sure about the difference between null or noop class and where's a good example to pick from,
so I stayed with null, but can revisit it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NoOp class will probably be the better option. Especially if you use classes rather than interfaces.
With classes, class hierarchy analysis optimizations will kick-in, so after JIT-ing, you still get a direct call with no type checks before the call.
But the more important thing is just to make the whole thing async
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 notes: I just realised I did not submit the main comment about my review 😅
Cleaning up the initialization was one of the reasons to use a Supplier in the first place
After Doug comment, serviceDiscoveryFactory() should even return the NOOP rather than null.
|
|
||
| maybeEnableServiceDiscovery(tracerBuilder); | ||
|
|
||
| installGlobalTracer(tracerBuilder.build()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| installGlobalTracer(tracerBuilder.build()); | |
| .pollForTracingConfiguration() | |
| .serviceDiscoveryFactory(TracerInstaller::serviceDiscoveryFactory) | |
| .build(); | |
| installGlobalTracer(tracer); |
where I would directly use a method reference to create the MemFDUnixWriter which can return null if the feature is disabled or unavailable.
@SuppressForbidden // intentional use of Class.forName
private static ServiceDiscoveryFactory serviceDiscoveryFactory() {
if (!Config.get().isServiceDiscoveryEnabled()) {
return null;
}
if (!OperatingSystem.isLinux()) {
log.debug("service discovery not supported outside linux");
return null;
}
// make sure this branch is not considered possible for graalvm artifact
if (Platform.isNativeImageBuilder() || Platform.isNativeImage()) {
log.debug("service discovery not supported on native images");
return null;
}
try {
// use reflection to load MemFDUnixWriter so it doesn't get picked up when we
// transitively look for all tracer class dependencies to install in GraalVM via
// VMRuntimeInstrumentation
Class<?> memFdClass =
Class.forName("datadog.trace.agent.tooling.servicediscovery.MemFDUnixWriter");
ForeignMemoryWriter memFd =
(ForeignMemoryWriter) memFdClass.getConstructor().newInstance();
return new ServiceDiscovery(memFd);
} catch (Throwable e) {
log.debug("service discovery not supported", e);
return null;
}
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar enough with factory patterns and can't get this to compile I'm sorry, would it be possible for you to commit directly what you have in mind here please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will do later this week 👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the benefit of introducing a method reference and always calling serviceDiscoveryFactory?
I could imagine you might want to defer all service-discovery logic, but IMHO the current approach is more readable and skips setting any factory when we know up-front that it's not possible or applicable.
Also note that if you do set a serviceDiscoveryFactory then that will always trigger a call in CoreTracer to schedule an async task via AgentTaskScheduler to evaluate that factory. On platforms where we know up-front that service discovery is not possible/applicable this will lead to an unnecessary call to schedule a task which will do nothing.
Given this I would leave this code as it is today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feature wise looks good to me. The implementation is not affecting performance indicators and can be opted-out.
@raphaelgavache can you please add deactivation instructions to the PR description?
Also, there are rooms for improvements raised by @PerfectSlayer that should be considered before merging this or addressed in a followup pr.
dd-trace-core/src/main/java/datadog/trace/core/servicediscovery/ServiceDiscovery.java
Outdated
Show resolved
Hide resolved
dd-trace-core/src/main/java/datadog/trace/core/servicediscovery/ServiceDiscovery.java
Outdated
Show resolved
Hide resolved
| return instrumenterConfig.isTraceEnabled(); | ||
| } | ||
|
|
||
| public boolean isServiceDiscoveryEnabled() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for adding a feature-flag to control this!
...gent-tooling/src/main/java/datadog/trace/agent/tooling/servicediscovery/MemFDUnixWriter.java
Outdated
Show resolved
Hide resolved
...gent-tooling/src/main/java/datadog/trace/agent/tooling/servicediscovery/MemFDUnixWriter.java
Outdated
Show resolved
Hide resolved
...gent-tooling/src/main/java/datadog/trace/agent/tooling/servicediscovery/MemFDUnixWriter.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for putting this together
Co-authored-by: Stuart McCulloch <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pushed factory refactoring
|
|
||
| NativeLong written = libc.write(memFd, buf, new NativeLong(payload.length)); | ||
| if (written.longValue() != payload.length) { | ||
| log.warn("write to memfd failed errno={}", Native.getLastError()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❔ question: Should we clear the memfd if write failed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is safe to have it partially written and open, so I think the less libc interaction the better
| public void write(byte[] payload) { | ||
| final LibC libc = Native.load("c", LibC.class); | ||
|
|
||
| int memFd = libc.memfd_create("datadog-tracer-info", MFD_CLOEXEC | MFD_ALLOW_SEALING); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just realised the agent implementation contrary to system tests matches on an additional -
adding it to the file name
Add support for service discovery using JNA, a second version using JNI will be replacing this approach when available
Can be disabled with
env var: DD_TRACE_SERVICE_DISCOVERY_ENABLED=true
system arg: dd.trace.service.discovery.enabled=true
Test instructions
On system-tests
To test on system-tests
On a linux VM
Check file descriptors
Sleep is a sleep 50s java class for test purpose
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]